feat: add notifications for account delegation by hamza221 · Pull Request #12935 · nextcloud/mail

hamza221 · 2026-05-14T09:37:01Z

Part of #12403

Unit tests Ai-assisted

Summary by CodeRabbit

New Features
- Account delegation now sends notifications to delegated users when they receive or lose delegation access to an account, including details about the delegating user and affected account.
Tests
- Added comprehensive test coverage for account delegation notifications, including notifier behavior and notification delivery for both delegation and revocation scenarios.

hamza221 · 2026-05-14T10:21:08Z

@coderabbitai review

coderabbitai · 2026-05-14T10:21:13Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2026-05-14T10:22:06Z

@hamza221 I'll trigger a review of the PR now. I also noticed the PR description image shows "delagation" in the notification subtitle — that appears to be a typo for "delegation". Worth checking if that string originates from a translation key or hardcoded text.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2026-05-14T10:24:55Z

📝 Walkthrough

Walkthrough

DelegationService now accepts Account objects and current user IDs, triggers notifications on delegation changes, and the controller, notification system, and tests are updated to support the refactored signatures and new notification behavior.

Changes

Account Delegation with Notifications

Layer / File(s)	Summary
DelegationService API and Notification Support `lib/Service/DelegationService.php`	Updated `delegate()` and `unDelegate()` to accept `Account` objects and current user ID instead of account IDs. Extended the constructor with `IUserManager`, notification `IManager`, and `ITimeFactory` dependencies. Added a private `notify()` helper that creates and dispatches `mail` notifications to delegated-to users with delegation state, delegator info, and account email.
Notifier account_delegation Handler `lib/Notification/Notifier.php`	Added `account_delegation` case to `Notifier::prepare()` that sets a delegation icon and populates rich subject/message content based on the `delegated` flag in notification parameters, distinguishing between active delegation and revocation.
Controller Integration `lib/Controller/DelegationController.php`	Updated `delegate()` and `unDelegate()` to pass the resolved `Account` object and `$this->currentUserId` to `DelegationService` instead of the numeric account ID.
Test Updates - Service and Controller `tests/Unit/Controller/DelegationControllerTest.php`, `tests/Unit/Service/DelegationServiceTest.php`	Updated mock expectations in controller tests to match the new method signatures. Refactored service test setup to inject user manager, notification manager, and time factory. Added notification-focused tests asserting that `delegate()` and `unDelegate()` send expected notification payloads with delegation state, delegator display name, and account email.
Notifier Test Coverage `tests/Unit/Notification/NotifierTest.php`	Added new test class verifying `Notifier` metadata, error handling for unknown subjects, and correct notification formatting for delegation-active and revocation scenarios with expected rich subject/message templates.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

nextcloud/mail#12727: Implements the account delegation MVP with similar core delegation entry points and service layer modifications.

Suggested labels

skill:backend

Suggested reviewers

ChristophWurst
kesselb
GretaD

Poem

A delegation dance, now refined with care,
Account objects flow through services fair,
Notifications bloom when permissions are shared,
Tests verify all, showing none was spared!
🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately describes the main feature addition: notifications for account delegation. It is concise, clear, and directly reflects the primary change across all modified files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/delegation/add-notifications

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

Generate code and open pull requests
Plan features and break down work
Investigate incidents and troubleshoot customer tickets together
Automate recurring tasks and respond to alerts with triggers
Summarize progress and report instantly

Built for teams:

Shared memory across your entire org—no repeating context
Per-thread sandboxes to safely plan and execute work
Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (1)

tests/Unit/Service/DelegationServiceTest.php (1)
177-189: ⚡ Quick win

Assert that no notification is sent when undelegation target does not exist.

This branch currently verifies deletion is skipped, but not that IManager::notify is skipped too.
Proposed test strengthening
 	public function testUnDelegateWhenNotFound(): void {
 		$this->mockNotification();
 
+		$this->notificationManager->expects($this->never())
+			->method('notify');
+
 		$this->delegationMapper->expects($this->once())
 			->method('find')
 			->with(1, 'delegatee')
 			->willThrowException(new DoesNotExistException('Not found'));

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d7d080e5-d6ca-4d23-92b8-6cc947971454

📥 Commits

Reviewing files that changed from the base of the PR and between 114629c and db6ddb3.

📒 Files selected for processing (6)

lib/Controller/DelegationController.php
lib/Notification/Notifier.php
lib/Service/DelegationService.php
tests/Unit/Controller/DelegationControllerTest.php
tests/Unit/Notification/NotifierTest.php
tests/Unit/Service/DelegationServiceTest.php

Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>

AI-assisted: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>

hamza221 added enhancement 3. to review labels May 14, 2026

hamza221 self-assigned this May 14, 2026

This was referenced May 14, 2026

Account delegation #12403

Open

feat: add notifications for account delegation #12728

Closed

hamza221 force-pushed the feat/delegation/add-notifications branch from cbb141d to e58a3f6 Compare May 14, 2026 09:58

hamza221 marked this pull request as ready for review May 14, 2026 09:58

hamza221 requested review from ChristophWurst, GretaD and kesselb as code owners May 14, 2026 09:58

hamza221 force-pushed the feat/delegation/add-notifications branch 3 times, most recently from 4c7c5b7 to db6ddb3 Compare May 14, 2026 10:20

coderabbitai Bot reviewed May 14, 2026

View reviewed changes

Comment thread lib/Notification/Notifier.php Outdated

Comment thread lib/Service/DelegationService.php

ChristophWurst approved these changes May 19, 2026

View reviewed changes

hamza221 added 2 commits May 19, 2026 12:25

feat: add notifications for account delegation

2e1ddae

Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>

test: cover delegation branches in Notifier

dc0799b

AI-assisted: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>

hamza221 force-pushed the feat/delegation/add-notifications branch from db6ddb3 to dc0799b Compare May 19, 2026 10:25

hamza221 enabled auto-merge May 19, 2026 10:26

hamza221 merged commit f549276 into main May 19, 2026
46 checks passed

hamza221 deleted the feat/delegation/add-notifications branch May 19, 2026 10:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add notifications for account delegation#12935

feat: add notifications for account delegation#12935
hamza221 merged 2 commits into
mainfrom
feat/delegation/add-notifications

hamza221 commented May 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

hamza221 commented May 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 14, 2026

Uh oh!

coderabbitai Bot commented May 14, 2026

Uh oh!

coderabbitai Bot commented May 14, 2026

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

hamza221 commented May 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

hamza221 commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented May 14, 2026

Uh oh!

coderabbitai Bot commented May 14, 2026

Uh oh!

coderabbitai Bot commented May 14, 2026

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hamza221 commented May 14, 2026 •

edited by coderabbitai Bot

Loading

hamza221 commented May 14, 2026 •

edited

Loading